Skip to content

Read the dtype from text_config in convert_ggml_to_int8.py - #2946

Merged
kevalmorabia97 merged 1 commit into
mainfrom
kmorabia/ggml-int8-text-config-dtype
Oct 22, 2026
Merged

kevalmorabia97 merged 1 commit into
mainfrom
kmorabia/ggml-int8-text-config-dtype

Conversation

@kevalmorabia97

@kevalmorabia97 kevalmorabia97 commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Type of change: Bug fix

convert_ggml_to_int8.py (#2903) read the scale dtype from the top-level dtype / torch_dtype of config.json. Multimodal configs such as GLM-5.3-Flash's keep it only in text_config, so converting a GGML export of GLM-5.3-Flash failed with KeyError: 'torch_dtype'. The converter now falls back to text_config.

Usage

Unchanged:

python convert_ggml_to_int8.py <ggml_dir> <int8_dir>

Testing

  • New test_dtype_is_read_from_text_config in tests/examples/hf_ptq/test_convert_ggml_to_int8.py moves the tiny export's dtype into text_config and converts it. It fails on main with KeyError: 'torch_dtype' and passes with the fix. All 4 tests in the file pass locally (the new one runs in 0.2 s).
  • End to end with this patch: converted a GLM-5.3-Flash IQ2_S GGML export (122 GB → 350 GB INT8, 5.5 min) and served it on vLLM 0.31 at TP4 on GB300. Per-token NLL on GPQA and AA-LCR responses matches the BF16 fake-quant export within 0.0013 nats in every length bin.

Before your PR is "Ready for review"

  • Is this change backward compatible?: ✅
  • If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: N/A
  • Did you write any new necessary tests?: ✅
  • Did you update Changelog?: N/A (the converter is unreleased)
  • Did you get Claude approval on this PR?: ❌

Additional Information

Follow-up to #2903.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Int8 conversion now correctly handles exported models whose data type is specified within their text configuration, preserving the appropriate precision for converted weight scales. This prevents conversion from failing when the data type is not provided at the top level.

Multimodal configs such as GLM-5.3-Flash's keep dtype only in text_config,
so the converter failed with KeyError: 'torch_dtype'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
@kevalmorabia97
kevalmorabia97 requested review from a team as code owners October 22, 2026 19:42
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between db4dc7a and abf9363.

📒 Files selected for processing: 2
  • examples/hf_ptq/convert_ggml_to_int8.py
  • tests/examples/hf_ptq/test_convert_ggml_to_int8.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

The converter now checks dtype fields in text_config after top-level dtype fields when selecting the scale dtype. A new test verifies conversion when the dtype exists only in text_config.

Changes: GGML int8 dtype fallback

Layer / File(s) Summary
Scale dtype selection and conversion test
examples/hf_ptq/convert_ggml_to_int8.py, tests/examples/hf_ptq/test_convert_ggml_to_int8.py
The converter checks top-level dtype and torch_dtype, then text_config.dtype and text_config.torch_dtype. The test verifies that an export with dtype only in text_config converts its weight-scale tensor to bfloat16.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: cjluo-nv

Merge Risk: ⚪ Minimal · up to abf93

No actionable merge-blocking risk remains.

Pre-merge checks: Passed 5 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 5 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: reading the dtype from text_config in convert_ggml_to_int8.py.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns Passed The pull request changes only examples/hf_ptq/convert_ggml_to_int8.py and its test. The added code contains no torch.load(..., weights_only=False), pickle-enabled NumPy loading, hardcoded `trust_r…
Full details: Security Anti-Patterns

Explanation

The pull request changes only examples/hf_ptq/convert_ggml_to_int8.py and its test. The added code contains no torch.load(..., weights_only=False), pickle-enabled NumPy loading, hardcoded trust_remote_code=True, eval, exec, or # nosec. It adds no dependency-file changes. No listed security anti-pattern is introduced.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@meenchen meenchen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot review (gpt-6.1-sol) — DM the bot to share feedback.

Approve: the fallback preserves top-level dtype precedence and fixes text-only dtype lookup, with a regression test checking the converted scale precision.

No action needed.

@kevalmorabia97
kevalmorabia97 enabled auto-merge (squash) October 22, 2026 19:50
@codecov

codecov Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.67%. Comparing base (db4dc7a) to head (abf9363).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2946   +/-   ##
=======================================
  Coverage   79.67%   79.67%           
=======================================
  Files         659      659           
  Lines       74466    74466           
=======================================
  Hits        59331    59331           
  Misses      15135    15135           
Flag Coverage Δ
examples-gpt-oss 13.48% <ø> (-0.07%) ⬇️
examples-hf_ptq 23.43% <ø> (+<0.01%) ⬆️
examples-llm_distill 13.56% <ø> (-0.07%) ⬇️
examples-llm_eval 17.22% <ø> (-0.03%) ⬇️
examples-llm_qat 17.43% <ø> (-0.10%) ⬇️
examples-llm_sparsity 16.81% <ø> (-0.04%) ⬇️
examples-specdec_bench 13.26% <ø> (-0.06%) ⬇️
examples-speculative_decoding 17.48% <ø> (-0.08%) ⬇️
examples-vllm_serve 17.44% <ø> (-0.04%) ⬇️
unit 61.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kevalmorabia97
kevalmorabia97 merged commit 55b4fd9 into main Oct 22, 2026
41 checks passed
@kevalmorabia97
kevalmorabia97 deleted the kmorabia/ggml-int8-text-config-dtype branch October 22, 2026 20:13
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-22 20:14 UTC

kevalmorabia97 added a commit that referenced this pull request Oct 28, 2026
…and QAD (#2915)

### What does this PR do?

[2/2], after #2917 (date-prefixed announcement filenames, merged).

Type of change: documentation

**Executive summary**

- **IQ2_S + NVFP4 matches FP8 with no training.** It uses IQ2_S on the
experts' gate and up projections and NVFP4 on their down projections,
3.21 bits per expert weight. The model is ~155 GB, against 328 GB for
FP8 (2.1x smaller), and its six-benchmark mean is within 0.1 points of
FP8.
- **IQ2_S is the smallest option.** At 2.56 bits per weight on all
expert weights, the model is ~130 GB (2.5x smaller), and its mean is 2.3
points below FP8. Most of the loss is runaway generation on long tasks.
- **QAD fixes long-context runaways.** 300 iterations on IQ2_S brought
AA-LCR from 70.0 to 74.0 (FP8 74.9) and cut answers hitting the token
budget from 6.9% to 0.6%. The mean gap shrank from 2.3 to 1.4 points.
- **We recommend the PTQ recipes.** QAD cost ~4,500 GPU-hours, didn't
restore knowledge recall, and IQ2_S + NVFP4 does better without
training. Both recipes still trail FP8 by ~2–3 points on AA-Omniscience.
Evaluations served BF16 fake-quant exports; IQ2_S can also be converted
to an INT8 weight-only checkpoint that vLLM serves at 86 instead of 148
GiB of weights per GPU (#2903).

Adds an announcement page (blog) for the GLM-5.3-Flash IQ2 study, with a
results figure in the Qwen3.6 announcement's style, a card on the
announcements index and a Latest News entry in the README. It presents
two PTQ recipes as alternatives:

| | FP8 checkpoint | IQ2_S | IQ2_S + NVFP4 |
|---|---|---|---|
| Routed experts | FP8 | IQ2_S | IQ2_S gate/up, NVFP4 down |
| Routed-expert bits per weight | 8 | 2.56 | 3.21 |
| Model size | 328 GB | ~130 GB | ~155 GB |
| Mean delta vs FP8, six benchmarks | – | −2.3 | −0.1 |

It also covers knowledge recall (HLE, AA-Omniscience), RULER-1M, QAD on
the IQ2_S model (300 iterations, −1.4, with worse recall), the cost, and
the GLM-specific reproduction steps.

**Blog, not a full tutorial.** QAD did not become the recipe: IQ2_S +
NVFP4 reaches FP8 with PTQ alone. So the page points to the
Qwen3.6-35B-A3B tutorial for the general flow and lists only GLM's
differences: the recipes, `--no_moe_grouped_gemm`, EP16 PTQ and
fake-quant export, long-reasoning calibration, eval settings, and the
QAD flags.

**Baseline:** every delta is against the FP8 release
(`zai-org/GLM-5.3-Flash`), not the BF16 checkpoint. A note near the top
says so.

**Sizes:** computed from each tensor's bits per weight using the FP8
checkpoint's safetensors headers, since evaluation used BF16 fake-quant
exports. The post also covers serving IQ2_S as INT8 weight-only
(`convert_ggml_to_int8.py`, #2903 / #2946): accuracy, weight memory and
vLLM throughput with the Humming kernels. IQ2_S + NVFP4 has no such path
yet.

**Dependencies:** none for merging this docs-only change. Reproducing
the experiments needed:
- ModelOpt: #2873 (merged; GGML PTQ, QAD and fake-quant export for large
MoE/VLM models) and #2539 (GLM-5.3-Flash support for Megatron-Bridge PTQ
and HF export, plus both IQ2_S recipes).
- Megatron-Bridge: NVIDIA-NeMo/Megatron-Bridge#6044 (GLM-5.3-Flash
bridge).
- Megatron-LM: #7521 (mHC precision controls) and #7522 (KPool DSA
indexer), both merged, and the open #7054 (KDA hybrid layers), plus two
training fixes for QAD: ceaa6c5d (#5835, distributed-optimizer save/load
for mixed fp32/bf16 param groups) and bfb17738 (#7522, freeze the DSA
indexer when `dsa_indexer_loss_coeff` is 0).
- transformers >= 5.16.1 (`glm5_next`), nemo:26.10 base image, vLLM >=
0.30 for evaluation.

The recipes stay in #2539: they use Megatron-Bridge module names, and
`examples/hf_ptq` would neither apply GPTQ to the fused HF routed
experts nor match the dense-MLP and NVFP4-down patterns, so they cannot
be used without the Megatron-Bridge support.

**Prefix-caching check (done):** our evals serve vLLM 0.31 with prefix
caching on, which on vLLM < 0.31 corrupts GLM's long-context cache
(vllm#57477). An A/B on FP8 AA-LCR (8 runs each) gives 74.88 ± 0.87 with
caching on and 76.12 ± 0.83 with it off: about one standard error apart,
so the 0.31 fix holds and the published numbers stand.

### Usage

N/A (documentation)

### Testing

- Parsed the new page with docutils: no errors or warnings.
- Did not run a full Sphinx build.

### Before your PR is "*Ready for review*"

- Is this change backward compatible?: N/A
- If you copied code from any other sources or added a new PIP
dependency, did you follow guidance in `CONTRIBUTING.md`: N/A
- Did you write any new necessary tests?: N/A
- Did you update
[Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?:
N/A
- Did you get Claude approval on this PR?: ❌

### Additional Information

Dated 2026-10-28, the publish date. The Reproducing section points at
Megatron-LM main (which now has #7521) with #7054 merged in, and at
Megatron-Bridge #6044; it will be updated once both merge.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants